Allow syntax extensions to return multiple items, closes #16723 #17236

fhahn · 2014-09-13T17:39:52Z

This is PR is basically the change proposed by @SimonSapin for #16723. I can also extend MacItem, as @huonw suggested, but I'm not sure how that was meant exactly. A comment for MacItem states that it's a convenience type for macros that return a single item. Should I adapt MacItem to contain a SmallVector of items instead of a single item and update the comment?

And I've two questions concerning the patch.

First, I am not sure what's the best way to duplicate the SmallVector holding the items. The way I'm doing it at the moment strikes me as suboptimal (https://github.com/fhahn/rust/compare/issue-16723-multiple-items?expand=1#diff-5100da09ad52c81568562e36ee359343R241). An additional map() call is required, because iter() returns an iterator to pointers to the elements. Is there a better way? Should I open a ticket and implement clone() for SmallVector?

And second, I had problems running the test in the test suite because it complains that there is no name MacItems in syntax::base::ext although there is. When I compile the file with

 LD_LIBRARY_PATH=x86_64-unknown-linux-gnu/stage2/lib/ 
 x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/bin/rustc 
 -L x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/ 
 src/test/auxiliary/issue_16723_multiple_items_syntax_ext.rs

it works. But I'm not sure which rustc binary should be used, e.g. there is on stage1 build in x86_64-unknown-linux-gnu/stage1/bin/rustc and one in x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/bin/rustc.

alexcrichton · 2014-09-15T17:43:10Z

src/libsyntax/ext/base.rs

This takes self by value now so this could just be Some(self.items.move_iter().collect())

Or just Some(self.items)

Now that MacItems does indeed store a SmallVector this should just return Some(self.items)

Ah, yes! Updated the PR.

alexcrichton · 2014-09-15T17:43:22Z

cc @sfackler

sfackler · 2014-09-15T17:45:37Z

You might as well remove MacItem entirely.

sfackler · 2014-09-15T17:47:15Z

src/test/run-pass/issue_16723_multiple_items_syntax_ext.rs

This needs to have an // ignore-stage1.

fhahn · 2014-09-15T20:42:51Z

Thanks for your feedback, I've updated the PR.

alexcrichton · 2014-09-17T16:54:01Z

src/libsyntax/ext/base.rs

This FIXME can likely be removed now

Yup, thanks. Does it still make sense to implement Clone on SmallVector?

sfackler · 2014-09-18T22:29:26Z

src/test/run-pass/issue_16723_multiple_items_syntax_ext.rs

This needs to go in run_pass_fulldeps

Thanks, that was the problem!

The test should pass now, thanks for your help and patience ;)

. This patch replaces `MacItem` with `MacItems`.

fhahn force-pushed the issue-16723-multiple-items branch 3 times, most recently from 79f55c7 to 5f0c2ea Compare September 14, 2014 13:26

alexcrichton reviewed Sep 15, 2014
View reviewed changes

sfackler reviewed Sep 15, 2014
View reviewed changes

src/test/run-pass/issue_16723_multiple_items_syntax_ext.rs Outdated

Copy link

Member

sfackler Sep 15, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to have an // ignore-stage1.

fhahn force-pushed the issue-16723-multiple-items branch from 5f0c2ea to 36b3079 Compare September 15, 2014 20:42

fhahn force-pushed the issue-16723-multiple-items branch from 36b3079 to 98d6501 Compare September 17, 2014 16:27

alexcrichton reviewed Sep 17, 2014
View reviewed changes

fhahn force-pushed the issue-16723-multiple-items branch from 98d6501 to f694386 Compare September 17, 2014 20:46

fhahn force-pushed the issue-16723-multiple-items branch from f694386 to 3fc46ea Compare September 18, 2014 09:00

sfackler reviewed Sep 18, 2014
View reviewed changes

Allow syntax extensions to return multiple items, closes rust-lang#16723

89b0944

. This patch replaces `MacItem` with `MacItems`.

fhahn force-pushed the issue-16723-multiple-items branch from 3fc46ea to 89b0944 Compare September 19, 2014 09:11

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Sep 19, 2014

rollup merge of rust-lang#17236 : fhahn/issue-16723-multiple-items

1686353

bors merged commit 89b0944 into rust-lang:master Sep 20, 2014

fhahn deleted the issue-16723-multiple-items branch September 20, 2014 19:53

Allow syntax extensions to return multiple items, closes #16723 #17236

Allow syntax extensions to return multiple items, closes #16723 #17236

Uh oh!

Conversation

fhahn commented Sep 13, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexcrichton commented Sep 15, 2014

Uh oh!

sfackler commented Sep 15, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fhahn commented Sep 15, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants